fix(claude): preserve valid reviews and reject failed executions - #546

Why

Claude Code can emit [claude-code:unrecognized_model] for a working gateway alias while returning a successful JSON result. Its official error reference describes this as a diagnostic that does not stop the request or switch models.

Upstream 9.3.2 (272a4020) still classifies the stderr marker as model unavailability before honoring the successful result. Flow discards the review text and rejects the result, or can enter the availability ladder when the model was not explicitly pinned.

Review focus

Start with run_claude_exec and _parse_claude_result in plugins/flow-next/scripts/flowctl.py, then the handler-level regressions in test_claude_review_commands.py.

A valid zero-exit success envelope takes precedence over the warning. Malformed envelopes, error results, nonzero process exits and timeouts still cannot supply a verdict. Genuine unavailable-model responses retain the existing bounded resolution behavior, and explicit model selections never silently change.

This existing PR also carries its previously reviewed shared fallback-cache correction: only successful fallback executions are cached, and failed cached selections are invalidated. The latest update adds two Claude regressions and merges upstream 9.3.2; it adds no new backend behavior, configuration or fallback.

Needs you

Repository-wide flowctl validate --all still reports 23 legacy errors in fn-16-ugn and fn-24-codex-first-class-citizen-native-plugin. The error set is identical on unmodified upstream 9.3.2. These historical records are outside this correction.

The upstream maintainer retains the merge decision. No hosted model review was requested.

Reproduce without a provider

The fake Claude process returns:

exit status: 0
stdout: {"type":"result","subtype":"success","is_error":false,"result":"<verdict>SHIP</verdict>","session_id":"11111111-2222-3333-4444-555555555555"}
stderr: [claude-code:unrecognized_model] {"model":"gateway-custom-model","query_source":"sdk"}

Run the real-handler regression:

PYTHONPATH=plugins/flow-next/tests python3 -m unittest test_claude_review_commands.ClaudeImplReview.test_ship_with_unknown_model_warning_keeps_explicit_model

With the regression transplanted onto unmodified upstream 272a4020, Flow returns 2 instead of the expected 0. On this branch it writes a SHIP receipt, retains claude:gateway-custom-model:medium, dispatches exactly once and creates no availability-cache entry. No LLM is invoked.

The negative cases cover malformed JSON and wrong-typed fields, genuine model errors, nonzero exits, and a timeout carrying partial SHIP output plus the warning. Failed attempts publish no verdict or new valid receipt, refund their reservation and do not silently fall back.

Verified

• Exact SHIP regression: red on unmodified upstream 272a4020, green with the correction.
• Focused Claude and model-resolution suites: 96 tests passed.
• python3 scripts/run_tests_parallel.py: 4,082 tests across 218 files; 0 failures, 0 errors, 7 skipped.
• uvx ruff@0.16.0 check ., tracker manifest freshness, Codex mirror freshness, documentation anchors and git diff --check: passed.
• Both contribution specs validate individually without errors or warnings.
• Independent packaged Flow review via codex:gpt-6.1-sol:xhigh: SHIP, no findings, finalized for candidate a0c9cab5 against upstream 272a4020. The reviewer completed its inspection and in-memory checks; its read-only sandbox prevented the temporary-directory suites, which passed in the coordinator's full run.
• Current GitHub CI will run on the pushed candidate; earlier CI results belong to the earlier head.

The historical Flow records remain bound to their recorded snapshots. The new review covers the current integrated candidate. This PR contains no private incident logs or application changes, and does not modify shared installations or permanent model routing.

Archived FN-13 offline replay

The original Claude JSON stdout was not retained. An offline replay placed the preserved complete result text and warning into a synthetic success envelope; the installed Flow-Next 9.1.1 adapter and unmodified upstream 9.3.2 rejected it, while PR head a0c9cab5 accepted it after one mocked dispatch. Synthetic negative cases covered genuine 404 model errors, nonzero transport exits, malformed JSON, missing fields, and error envelopes. This replay invoked no LLM and created no official review receipt.